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You should avoid these worst practices—and eliminate them when 
you maintain or refactor existing code. 


Every so often, you see code that someone else has written—or code that you wrote—and smack your 
head in wonder, disbelief, and dismay. 


My previous article, “Ten Java coding antipatterns to avoid: Worst practices #10 through #6,” explores five 
of those antipatterns. I'll conclude the discussion here with the final five worst practices, plus a bonus. 


I'll reiterate what | wrote in the previous article’s introduction: You should avoid these worst practices—and 
eliminate them when you maintain or refactor existing code. And, of course, resolve them if you see these 
issues during a code review. 


Worst practice #5: Duplicating code 


Many developers are taught early on that copy-and-paste is a bad idea. Literally copying code from 


elsewhere in an application is bad because it creates a maintenance nightmare: Finding a bug or changing 
the functionality requires that you find all the copies and fix them all. Copies are also bad because they 
make a program needlessly larger. 


Many IDEs have “extract method” or “introduce method” refactoring functions that take existing code and 
turn it into anew Java method. If you create a method instead of copying and pasting, your code will be 
shorter, clearer, and cleaner, as well as easier to debug and maintain. CPD, the copy-and-paste detector 
from the PMD Open Source Project, is a useful tool for finding where copy-and-paste has been applied. It 
uses a clever algorithm to find duplicated tokens, and by default it looks for a run of 100 or more tokens, 
most of which must be identical to be declared a copy. A token is an element such as a keyword, literal, 
operator, separator, or identifier. 


CPD is distributed as part of PMD, which is an extensible cross-language static code analyzer. 


One of my open source GitHub repositories contains all the code examples from my Java Cookbook plus 
many other code samples. Unfortunately, some of the examples not used in the book do not get the 
regular maintenance they deserve. 


(In my defense, sometimes a developer does copy a code example for legitimate reasons that wouldn't 
apply when building a real application.) 


While writing this article, | ran CPD against my repository, and it found several issues. Here are two. 


$ cpd 
Found a 14 line (184 tokens) duplication in the following files: 


Starting at line 19 of /home/ian/git/javasrc/desktop/src/main/java/gui/FlowLa 
Starting at line 37 of /home/ian/git/javasrc/desktop/src/main/java/qui/FlowLa 
getContentPane().add(quitButton = new JButton ("Stop") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton("Exit")); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton ("Exit") ); 
getContentPane().add(quitButton = new JButton("Exit")); 


Copy code snippet 


The first one is interesting. It is obviously an editing error; when you use the vi editor, a number followed 
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letter G (for go) are used to jump to a line by number. 


My guess is that | typed a number to jump to a line, forgot the G, and typed a line to be inserted at that 
location, causing the line to be erroneously inserted many times. Strangely, this mistake has been in my 
public repository since 2003, and nobody has ever reported it to me. 


The second issue identified an 18-line (184 tokens) duplication in the following files: 


line 28 of /home/ian/git/javasrc/main/src/main/java/regex/LogRegE 
line 25 of /home/ian/git/javasrc/main/src/main/java/regex/LogRegE 
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System.out.printin("Input line:" + logEntryLine) ; 


Matcher matcher = p.matcher (logEntryLine) ; 


if (!matcher.matches() || 
LogParseInfo.MIN FIELDS > matcher.groupCount()) { 
System.err.println("Bad log entry (or problem with regex):"); 


System.err.println(logEntryLine); 


rerums 
} 
System.out.printlin("IP Address: " + matcher.group(1)); 
System.out.printin("UserName: " + matcher.group(3)); 
System.out.println("Date/Time: " + matcher.group(4)); 
System.out.printin("Request: " + matcher.group(5)); 
System.out.printlin("Response: " + matcher.group(6)); 
System.out.println("Bytes Sent: " + matcher.group(7)); 
Wie lineecner Grotio (e) « Senulelliss (MEN), 

System. Out peineln( "Rereren M marcher group (Shi); 
System.out.printin("User-Agent: " + matcher.group(9)); 


he 


Copy code snippet 


The same program demonstrated the use of regular expressions to parse the common Apache Log File 
format, and it seems as if | somehow accidentally created the same file with two different names, perhaps 
while merging files into this repository from another. 


Here | am, rightfully busted by a tool that | often recommend. | shall have to use CPD more often. 


Worst practice #4: Out-of-date Javadoc 


Javadoc is your friend—but having friends takes work. To be able to read documentation and apply it 
usefully, it must be up to date. Therefore, when you change the arguments to a function, for example, you 
need to change the Javadoc accordingly. Don’t be the developer responsible for the following: 


[** 
* Perform the aFunction function. 


* @parameter x The X coordinate to start 


* @parameter y The Y coordinate to start 
* @Parameter len The number of points to process 
af 
public double aFunction(double x, double y, double endX, double endY) { 


Copy code snippet 


Your Javadoc can be more useful if it is generated in formats such as HTML, for reference. Maven and 
Gradle and other build tools have plugins that make it easy to generate Javadoc web pages as part of your 
overall build process. In Maven it may take 10 or 15 lines of plugin configuration to tame Javadoc, but once 
that's written, that configuration rarely changes. 


You may want to include the following configuration element when getting started: 


<failOnError>false</failOnError> 


Copy code snippet 


By the way, old and sporadically maintained Javadoc will otherwise fail the build completely. This gives 
you time to clean up the documentation incrementally and get it into a condition you'll be proud to show 
to other developers. 


Worst practice #3: Unvalidated user input 


In 1979, Brian Kernighan and P.J. Plauger wrote a book called The Elements of Programming Style. 
Although it was written with some older programming languages for the examples, Kernighan and 
Plauger’s book contains much developer advice that is truly timeless. One of my favorite idioms is 


Never trust user input. 
Well, they actually wrote, “Test input for plausibility and validity,” but | like my formulation better. 


When reading code in which someone has written JDBC calls, it is not uncommon to find this antipattern 
in the first statement. 


rs = jdbcConnection.createStatement ().executeQuery( // bad 
"select * from customers where nam UU  nameriellol Getiexte ()) + WY pw) g 


PreparedStatement statement = jdbcConnection.prepareStatement ( 


"Select * from customers where name = ?1"); // better 


statement.setString(l, nameField.getText()); 


rs = statement.executeQuery (); 


Copy code snippet 


The value of nameField.getText() is almost certainly coming straight from the user; that is, it is user 
input that should never be trusted. However, this data is being fed directly into the database. 


What happens if a bad actor enters the following as input, as illustrated in “327: Exploits of a mom”? 


Jorm Smirne Croo table Customere? == 


Copy code snippet 


It will be as though you had entered the following SQL: 


"select * from customers where name = 'John Smith'; drop table customers; --' 


Copy code snippet 


Many, if not most, JDBC drivers allow more than one statement on a line, with a semicolon (;) between 
each. Now what if the database architect was as careless as the developer? By not restricting the database 
account used by the app server from having drop privileges, it’s game over. 


The -- at the end is the twist of the knife because it will stop the leftover delimiter characters from even 
causing a syntax error in the log file, obfuscating where the vandalism occurred. 


Java’s PreparedStatement interface obviates this problem: This interface will treat the entire string 
(whether it’s normal or malicious) as characters to match in the where clause, and if the input is bad, it will 
fail safely. 


SQL injection attacks such as this happen probably every day on small sites, so much so that they have 
been in the Open Web Application Security Project’s notorious OWASP Top 10 list since its inception. 


Worst practice #2: Not testing the not-unit-testable 


| dread walking into an old-school project that lacks unit tests. 


Many older applications were written in a single class, sometimes termed a ball of wax or all-in-one class 
or even a monster class. (There are even less-polite names.) It is difficult to write unit tests for monster 
classes because unit tests, by definition, are designed to test one small unit of code. A monster class has 
no small units of code to test! Not only are there no tests, but the code is also not written to be testable. 


If you are tasked with maintaining such an application, start carving the monster into smaller pieces that 
can be tested. How big or small should the code classes be? That’s a topic for endless debate, as there is 
no magic size and no exact number of lines of code for classes or for methods. 


The single-responsibility principle (SRP) says that each class should have one primary responsibility: 
performing some calculations, processing an order, or displaying the results. In other words, if your 
application does those three things, you need at least three classes. Similarly, SRP says that a method 
should do one thing, and one thing only. 


While you extract code out of the monolith, write unit tests—and make sure they pass. 


Of course, if you’re starting a project from scratch, you can have the benefit of writing the tests as you 
write the code. Writing the tests first—that is, following the test-driven development (TDD) methodology 
—allows the IDE to generate the outline of the class and methods being tested, guaranteeing that they are 
in sync from the beginning. 


Worst practice #1: Empty and undocumented catch blocks 


What does the following code do? 


Connection jdbcConnection = getConnection(); 


var sqlQuery = "select id,name from student where name like 'Robert%'"; 
ResultSet rs = null; 
try { 
try {rs = jdbcConnection.createStatement().executeQuery (sqlQuery) ; 
} catch (SQLException e) {} 
while (rs.next()) { 
rare iel = rs- Gecme (il): 
String name = rs.getString(2); 
logger. logici ae W als ap meme)? 
} 
} catch (SQLException e) { 
loggers log ("SO merero, €)? 


Copy code snippet 


The result depends on whether the first SOL operation succeeds. If the operation fails, the exception is 
swallowed—and nobody is the wiser. A few lines later, the code will get in trouble again, because ignoring 
the error is not a good strategy. 


This example is distilled down from actual code in a library (whose name | have no wish to remember) that 
our team used in a project | worked on years ago. However, in the real library, the illicit exception 
swallowing and the failing code were a few hundred lines apart, and the whole mess was down about 20 
levels in the library call stack. It took hours and hours to find this mess. 


The bottom line is that exceptions should never be caught and ignored. Either catch the exception or 
don't. If you do catch an exception, do something with it. At the very least, log the exception. If an 
exception is serious, either rethrow it or get out of the whole section of code that is in trouble. 


Many modern frameworks (such as Spring and Jakarta) that deal with JDBC will catch checked SOL 
exceptions and rethrow them as unchecked exceptions. This allows you to process them as close to the 
human user as possible, instead of requiring rows and rows of try-catch statements or throws clauses. 


The one exception to this rule of not ignoring exceptions is Thread.sleep, which has a checked 
InterruptedException. In single-threaded code, it may be permissible to ignore the exception if you 
comment it. 


tey | 
Threads DO AM EC IIHR SINC) F 

} catch (InterruptedException ex) { 
// Can't Happen - single threaded 


Copy code snippet 


Bonus worst practice: Ignoring warnings 


Address warnings from your IDE as they appear. Compiler warnings are a mixed bag: Sometimes they 
indicate serious bugs, but many times they are irrelevant, and some experience is needed to hone your 
warning judgment. 


Irrelevant warnings can often be squelched with the correct use of the @SuppressWarnings annotation. 


By contrast, relevant warnings need to be fixed immediately, because if you let warnings build up, odds 

are that your team will get in the habit of ignoring them. And then, someday when you least expect it, a 

real bug will slip through into production, and in the postmortem, somebody will notice that the IDE had 
been warning about it for weeks. 


Worse, once a project gets above some threshold of warnings, it is too late. You will probably never fix 
them. 


Code quality matters. Please keep your code clean. The developer job you save may be your own. 


Dig deeper 


e Ten Java coding antipatterns to avoid: Worst practices #10 through #6 


e Five code review antipatterns 


e Annotations: An inside look 


e Designing and implementing a library 


e Unit test your architecture with ArchUnit 


e Refactoring Java, Part 1: Driving agile development with test-driven development 


lan Darwin 


lan Darwin is a Java Champion who has done all kinds of development, from mainframe applications and 
desktop publishing applications for UNIX and Windows, to a desktop database application in Java, to 
healthcare apps in Java for Android. He’s the author of Java Cookbook and Android Cookbook (both from 
O’Reilly). He has also written a few courses and taught many at Learning Tree International. 
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